Skip to content

DEV-1703: typed-pipeline branch → dev-1450 (latest: merge origin/main) - #263

Merged
ZmeiGorynych merged 709 commits into
egor/dev-1450-principled-redesign-of-syntaxfrom
egor/dev-1703-comprehensive-approach-expressions-crossing-joins-on-the
Aug 5, 2026
Merged

DEV-1703: typed-pipeline branch → dev-1450 (latest: merge origin/main)#263
ZmeiGorynych merged 709 commits into
egor/dev-1450-principled-redesign-of-syntaxfrom
egor/dev-1703-comprehensive-approach-expressions-crossing-joins-on-the

Conversation

@ZmeiGorynych

Copy link
Copy Markdown
Member

Integration PR for the DEV-1703 typed-pipeline branch into egor/dev-1450-principled-redesign-of-syntax. The stage/parity sub-work (DEV-1704/1715/1716/1717) landed via its own PRs into this branch; the latest change is the origin/main merge below.

Latest change — merge origin/main

Integrated 18 origin/main commits into the typed pipeline:

  • DEV-1718 RLS ruleset refactor — auto-merge produced the correct JoinFilterRuleset adaptation in query_engine._policy_has_join_rules (incl. the and ruleset.joins guard); a full-tree sweep found no stale old-shape (data_filters / ColumnFilterRule) usages.
  • extensible help + run_queryquery — no branch references, no-op.
  • PR Unknown column type #259 "Unknown type" opaque columnsabsorbed main's opaque-dimension guard into the typed pipeline (stage_planner._declared_measures_from_query): rejects GROUP BY on DataType.UNKNOWN dims, firing on the declared type before bind_expr (so an opaque derived column is caught by type rather than tripping DEV-1410 cycle detection first).
  • CLAUDE.md declutter — adopted main's decluttered version + new DECISIONS.md (typed-pipeline convention bullets remain documented under docs/architecture/*).

Conflicts resolved (2)

  • CLAUDE.md → main's decluttered version.
  • tests/test_sql_generator.py → import union (AggRenderSpec + _wrap_cast_for_type).

Post-merge test triage — real fixes, no new xfails

  1. test_slayer_help_package_is_deleted → removed the stale untracked slayer/help/ bytecode dir.
  2. test_opaque_column_rejected_as_dimension → implemented the guard above.
  3. test_opaque_column_emits_no_cast_when_projected → aligned the over-broad "CAST(" not in assertion to "AS UNKNOWN" not in (typed pipeline intentionally casts the INT count per DEV-1361).

Verification: full non-integration suite green (7891 passed, 8 skipped, 98 xfailed — unchanged); ruff clean.

🤖 Generated with Claude Code

ZmeiGorynych and others added 30 commits July 13, 2026 16:46
…_server-help-seeding-crashes-schema

DEV-1669: guard + best-effort help-seeding in create_mcp_server
…osi-semantic-layer-configs-into-slayer-configs

# Conflicts:
#	slayer/engine/query_engine.py
Self-contained, fully offline worked example: builds a tiny retail DuckDB,
imports shop.osi.yaml via OsiToSlayerConverter, and runs five queries (simple,
multi-hop join, derived, cross-dataset, and multi-hop metrics) each verified
against gold SQL. Wired into the docs nav and gitignore.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- parser: coerce numeric OSI `version` (unquoted `1.0` parses as YAML float)
  to str so a valid-but-unquoted version isn't silently skipped (Codex).
- cli: catch OsiConversionError from import-osi (duplicate dataset names) and
  exit cleanly instead of dumping a traceback (Codex).
- demo notebook: factor the repeated "SLayer: ... gold: ..." prints into a
  show() helper to clear Sonar S1192 duplicated-literal issues.
- tests for the unquoted-version parse and the duplicate-dataset CLI exit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The duplicate-dataset CLI test nested _args() inside the pytest.raises
block, giving it two throwable invocations. Build args first, matching the
sibling exit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a (Codex)

The Clean-Fail Report section claimed the raw construct is preserved in meta,
but the converter only preserves ai_context/custom_extensions of successfully
converted entities; clean-failed constructs surface in the conversion report.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Report entries list entity/reason/workaround (not the raw SQL). A CASE only
clean-fails at the top level; COUNT(CASE ...) materializes via a hidden column,
so scope the example to a top-level CASE or a window function.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e-fixes

SLayer in general:
- Optional query timing measurement.

Pg facade:
- Fixed column quoting in WHERE .
- Return query errors properly.
Second notebook (osi_import_agent_nb.ipynb) walks the OSI import the way an
agent experiences it: two 'slayer' CLI commands (datasources create + import-osi,
via SLAYER_STORAGE) to ingest, then the in-process MCP tools (models_summary,
inspect, search, query) to explore and verify against gold. Includes how to
register SLayer as an MCP server in Claude.

Move the shared reference answers into setup_osi.compute_gold, consumed by both
notebooks; refactor the library notebook to use it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the cluttered intro with numbered steps (install, SLAYER_STORAGE,
create datasource, import-osi, MCP-in-Claude-Code, then the notebooks). Keep
'What the import produces' and 'The five queries'; drop the 'Gold checks up
front', 'The one-liner', and 'Further reading' sections (key links folded into
the intro).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ic-layer-configs-into-slayer-configs

DEV-1643: Import OSI (Open Semantic Interchange) configs into SLayer
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…facade-schema

Refresh Postgres facade schema
…profile-fixes

Sample values in column inspect only, not in ingest
A model/column/alias named after a SQL reserved word (grant, order, user,
group, select, ...) was unqueryable: the query builder derived the table
alias from the model name and emitted it — plus every column qualifier —
unquoted, so Postgres rejected the bare reserved word.

Two mechanisms, both keyed off one curated SLAYER_RESERVED_KEYWORDS set:

- install_reserved_keywords() unions the set into every dialect generator's
  RESERVED_KEYWORDS, so sqlglot's identifier_sql quotes reserved-word
  identifiers built as AST (base FROM alias + qualifiers, cross-model CTEs,
  physical names) at emit time. sqlglot's own per-dialect set is empty for
  Postgres/T-SQL/SQLite/ClickHouse/Snowflake/Databricks/Spark/Oracle.
- prequote_reserved_identifiers() token-quotes reserved words in qualifier
  (word.) / leaf (.word) position in a SLayer-generated string before it is
  re-parsed (bare reserved words fail at parse time, which emit-time quoting
  can't reach). Applied in SQLGenerator._parse / _parse_predicate (join_cond,
  measure.filter_sql, qualified WHERE incl. joined grant.status, first/last
  ranked subquery) and in column_expansion's pre-generator Column.sql parses.
  Token-based so literal/comment/quoted-ident safe; quotes for the parse
  dialect.

Two non-dot-adjacent AS <alias> string sites are quoted explicitly:
_maybe_quote_qualifier (first/last join alias) and a reserved-aware inline
quote in query_engine._query_as_model (query-backed short column).

Covered by 54 unit tests (guard, byte-level alias/qualifier across 8 dialects,
joins, first/last, filters, HAVING, raw-row, derived columns, token-helper
literal safety) and 4 live-Postgres integration tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex flagged that the curated SLAYER_RESERVED_KEYWORDS omitted reserved
words such as `between` that still fail as a bare alias. A probe over the
sqlglot keyword universe (PG tokenizer + every dialect's RESERVED_KEYWORDS)
surfaced 14 missing words that fail `SELECT w.x FROM t AS w` in Postgres:
alter, between, drop, glob, insert, out, partitioned_by, qualify, regexp,
revoke, rollback, uncache, xor. Added all (quoting is always safe) plus a
completeness regression test that re-runs the probe against the set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Sonar S3776)

Extract the per-token qualifier/leaf eligibility decision into a
_reserved_dot_edit helper so prequote_reserved_identifiers drops from
cognitive complexity 16 to within the 15 budget. Behavior is identical;
covered by the existing 52 unit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l (DEV-1686, Codex review)

Codex found that a derived column referencing a reserved joined model
(e.g. `bumped = "grant.amount + 1"` on a non-reserved root joined to `grant`)
generated SQL that referenced `"grant".amount` WITHOUT the required
`JOIN "Grant" AS "grant"` — so it failed at execution. The prior test only
asserted parseability and missed it.

Root cause: `_collect_needed_paths` scans the *expanded* dimension SQL with
`_TABLE_COL_RE` (bare `word.word`) to discover join paths, but RESERVED_KEYWORDS
emits the reserved qualifier quoted (`"grant".amount`), which the bare regex
never matched. Made `_TABLE_COL_RE` quote-tolerant — a strict superset of the
old pattern (unquoted refs and `__`-path aliases match unchanged; group(1) is
still the unquoted qualifier). Also prequote the enrichment join-path /
filter-atomicity parses and the save-time column-dependency parse so reserved
qualifiers parse cleanly instead of a noisy `Command` fallback.

Tests: the derived-column unit test now asserts the emitted JOIN byte-level,
and a new Postgres integration test executes a derived column that references
the reserved joined model and checks the result.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, Codex review)

Codex found the quote-tolerant _TABLE_COL_RE only handled ANSI double quotes,
so a reserved joined-model qualifier emitted with the dialect's own quote char
(`grant` on MySQL/BigQuery, [grant] on T-SQL) was still missed by join-path
discovery, dropping the JOIN on those dialects. Widened the regex to tolerate
every dialect's identifier quote char (still a strict superset of bare
word.word; group(1) stays the unquoted qualifier). Added a multi-dialect
regression test asserting the JOIN survives on postgres/mysql/bigquery/tsql/
sqlite/duckdb.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r/Codex)

- Clear the Sonar new-code duplication gate (6.3% → under 3%): merge the two
  near-identical _generate/_generate_via_engine helpers into one `_gen`, and
  extract the repeated derived-column model setup into
  _orders_derived_grant_storage.
- Document the known limitation Codex flagged: a physical column whose bare name
  is a statement-initial keyword (grant/select/insert/...) referenced UNQUALIFIED
  in a compound expression is not auto-quoted (sqlglot parses it as a statement
  even in expression context, and quoting non-dot-adjacent reserved words would
  corrupt genuine keywords). Trivial and qualified forms work.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match the project convention (keyword arguments for functions with more than
one parameter) across all prequote_reserved_identifiers call sites. No behavior
change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unquoted-table-aliases-breaking-any-model

Quote SQL reserved-word identifiers in generated SQL (DEV-1686)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ZmeiGorynych and others added 28 commits August 4, 2026 15:44
Conflicts resolved:

* DECISIONS.md — append-only log, so both sides kept; the base branch's
  DEV-1728 entry is placed before this branch's two DEV-1732 entries (it
  landed first).

* tests/test_sql_generator.py — both branches deduplicated the same SQL-shape
  helpers, in different directions. DEV-1728 moved `_norm` /
  `_extract_cte_body` into the new `tests/_cross_model_chain.py`; DEV-1732
  moved `_norm` / `_join_aliases` / `_extract_src_body` / `_extract_cte_body`
  into `tests/_engine_helpers.py`. Resolved toward one definition each:
  `_engine_helpers.py` is the home for the helpers that are not
  cross-model-specific, and `_cross_model_chain.py` imports and re-exports
  `_norm` / `_extract_cte_body` from it (already in its `__all__`, so its
  importers are unaffected). That is the same one-definition rule
  `_cross_model_chain.py`'s own docstring states, applied one module up —
  rather than reintroducing the byte-identical copies either branch removed.

Full non-integration suite green after the merge: 8653 passed, 5 skipped,
6 xfailed. Ruff clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Mode-A scalar string substitutes UNQUOTED — the template author writes
the quotes, which is what makes `amount >= {floor}` and `{d}::TIMESTAMP`
expressible. That rule presumes an author who can see the SQL position,
and the Cube importer's generated `col IN ({var})` template has none: the
parentheses are machine-written, so a caller passing "Acme" rendered
`IN (Acme)` — a column reference sqlglot parses happily and the database
then rejects, or silently resolves against a real column.

The converter now marks such variables `list_valued` in
`meta.cube_variables` (a front-end-neutral flag, not Cube's `kind`
taxonomy), and the engine wraps a bare scalar into a one-element list at
the single Mode-A choke point `_substitute_model_sql_surfaces`, which both
execution and the type-probe route through. Only str/int/float/bool are
wrapped; an empty list still raises, and None/dict keep their own error.
Hand-written models declare nothing and are untouched.

Also from review:

- `declares_variables(model)` now defeats the DEV-1625 zero-variable fast
  path via a shared `_model_needs_substitution_pass` predicate, closing
  the hole for a generated model whose pushdowns are all required (no
  `{? ?}` block to force a pass) — it used to emit a bare `{var}` into
  SQL instead of raising. Hand-written models keep the brace-literal
  protection (`'{1,2,3}'`).
- `list_valued` is matched with `is True`, not truthiness: `meta` is
  user-extensible, so a stray `1` or the string "false" must not switch
  substitution semantics.
- CLAUDE.md said Mode-A escaping was "not dialect-aware", which DEV-1727
  made false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Narrows DEV-1712 (Stage 8) to one rule: Law 1 pulls whatever joins an
ORDER BY ref crosses into the scope owning its rows -- even when ORDER BY
is the sole referencer -- and the ref is emitted in whatever form that
scope makes legal. Two of Stage 8's rejections become resolutions.

* LOCAL row column, GROUPED query: materialises a hidden `<col>:max`
  aggregate and orders on its alias. MAX is order-preserving per group
  and portable across every Tier-1 dialect. A TimeTruncKey wraps its
  UNDERLYING column (DATE_TRUNC is monotonic, and a TimeTruncKey is not
  a legal aggregate source).
* JOINED row column, RAW-ROWS query: pulls its join and split-emits
  `customers__regions.name`.

The wrap is interned post-bind, so the bind-time aggregation gate (PK
columns, allowed_aggregations, per-type defaults) deliberately does not
apply -- the caller asked to SORT a column, not aggregate it.

Two mechanical details worth flagging for review:
* interning runs after `_bucket_slots`, so the buckets are recomputed
  whenever a wrap is minted, otherwise the new slot never reaches
  PlannedQuery and the ORDER BY is silently dropped;
* `_collect_joined_paths_for_base` now also walks ORDER BY targets. An
  order-only joined column is deliberately NOT added to
  `base_render_order` -- that would project it and widen the GROUP BY
  grain -- but its join must still be bound in the base FROM.

Two rejections REMAIN by design, tracked as DEV-1735: a GROUPED query
with a JOINED sort key (an AggregateKey with a non-empty source.path
always routes to a target-rooted CTE, which for a host-grain sort key
degenerates to a scalar CROSS JOIN -- every group gets the same global
value and the sort silently does nothing, strictly worse than a clear
error), and an order-only LOCAL DERIVED column whose Column.sql crosses.
The three TestFlavorAJoinedOrderByDeferred xfails are re-pinned to
DEV-1735 with the reason rewritten to explain why.

Grain preservation -- a sort key must never reach GROUP BY -- is now
pinned explicitly rather than implied by a rejection. Correctness is
verified by executed values on SQLite and real Postgres, not just
emitted SQL shape.

Full non-integration suite: 8653 passed, 5 skipped, 6 xfailed. Ruff
clean. Integration: postgres 63, sqlite+duckdb 165 passed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sonar (11 OPEN issues; the quality gate passed, so the gate verdict alone
would have missed all of them):

* S3776 cognitive complexity on the three functions this PR grew. Extracted
  rather than suppressed, because each had a real extraction:
  - `ProjectionPlanner.plan` (24) repeated the same
    "intern as hidden unless already present" block FOUR times (measure aux
    deps, filter operands, order operands, and the DEV-1733 order-target
    composite) -> one `_intern_hidden` helper, which also stops the four
    sites drifting on declared_name/phase.
  - `ValueRegistry.intern` (18) -> alias-collision validation split into
    `_validate_alias_collisions`, leaving intern as validate -> merge-or-create.
  - `_build_windowed_plans` (18) -> grain-role partition split into
    `_windowed_grain_partition`.
* S9073 x6: split `assert A and B` into separate assertions. These sit on the
  load-bearing B1/B2 guards, where knowing which half failed matters.
* S5778: hoisted the `SQLGenerator(...)` constructor out of `pytest.raises`.
* S5958: narrowed `pytest.raises(Exception)` to `UnknownReferenceError`.

CodeRabbit:

* Nitpick (valid): the "no plain SUM in `_base`" assertion used
  `sql.split("), _wm_", 1)[0]`, so an emitter spacing change would return the
  whole statement and silently stop testing `_base`. Replaced with an
  AST-based `_cte_aggregate_sql` helper that selects the CTE by alias,
  matching the module header's own "walk the AST, never match on formatting"
  rule. This is the second half of the B1 guard — the one that catches the
  composite reading a plain aggregate while the `_wm_` CTE sits joined but
  unused — so a vacuous pass there would hide the regression. Verified
  non-vacuous with a positive control.
* Thread (invalid, replied): removing `@pytest.mark.integration` from
  `TestHiddenOrderOnlyWindowedValues` would diverge from the identical
  sibling class in the same file and from the `-m integration` split
  documented in CLAUDE.md.

Codex: no findings. CI: no failed checks. Behaviour unchanged — 8638 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-ups on 9bb846e:

- Codex: any non-empty `meta.cube_variables` counted as "importer-generated"
  and so disabled the DEV-1625 zero-variable brace-literal fast path. `meta`
  is free-form user data, so a hand-written bag reusing that key would make a
  model with raw braces (`'{1,2,3}'`) start raising on a query that used to
  work. An entry now counts only when it carries a string `member` — the shape
  every importer writes — so the bag identifies itself.
- Sonar python:S5778: hoist `SqliteDialect()` out of the `pytest.raises` block
  so only one call inside it can throw.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage C2 — every test file except tests/test_agg_render_spec_shim.py (which
Stage 11 deletes outright) is now off slayer.engine.enrichment/enriched, and
the last production caller is rewired. Prepares the ~3300-LOC deletion.

Production:
* memories/resolver.py used enrichment._collect_reachable_agg_names; switched
  to the behaviourally identical agg_registry.collect_reachable_agg_names.
  Coverage confirmed by sabotage: test_entity_resolution.py's DEV-1500 case
  fails if the call is neutered.

Two real bugs found by the migration and fixed (not deferred):

* T-SQL emitted CTEs INSIDE a derived table. The typed path string-built
  "FROM (<chain>) AS _outer" instead of delegating to
  SqlDialect.emit_outer_wrap, bypassing the DEV-1571 hoist. SQL Server allows
  WITH only as a statement prefix and rejects the result outright, so every
  T-SQL query with a transform chain was invalid. Both sites now route through
  the dialect hook via _emit_planned_outer_wrap; output on other dialects is
  unchanged.
* first/last ranked path emitted a mixed-case local dimension unquoted in the
  outer SELECT/GROUP BY while quoting it in the ranked subquery, so Postgres
  folded it to lowercase -> UndefinedColumn (DEV-1645 Flavor B). The dim now
  builds through _to_ident like every other construction site.

Test-side judgement calls, each recorded in the file:
* byte-equivalence goldens re-captured from the typed pipeline. Every diff was
  classified against a legacy-vs-typed diff first; all fell into result-type
  CASTs, DEV-1708 null-safe join-back, CTE structure, or dialect alias
  mangling. Nothing unexplained was blessed.
* log-alias expectations now name ORDERS.AMOUNT — the typed pipeline anchors
  refs at the scope root, and the qualified needle keeps the negative
  assertions meaningful.
* date_trunc "no CAST" check re-scoped to the time-dim operand; a blanket
  check now trips on the unrelated measure cast.
* ClickHouse LOWER/SUBSTR uppercased, verified executable on a live server by
  a new integration test rather than assumed.

Coverage was mapped before deleting legacy-only test classes; parse/bind
behaviours with no typed equivalent were backfilled into test_syntax.py,
test_binding.py and test_transforms_planner.py.

Full non-integration suite: 8580 passed, 5 skipped, 20 xfailed. Ruff clean.
The 14 new xfails pin a gap Phase 3 must close first: distinct_dimension_values
compile-time validation does not exist on the typed pipeline (see next commit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DEV-1543's contract is that `distinct_dimension_values=False` (raw rows,
no auto-dedup) rejects every measure reference. SlayerQuery's validator
covers the model-free half (non-empty `measures`, no dimensions); the
half that needs the resolved model -- references hiding in `filters` and
`order` -- lived only in the legacy enrichment module and was never
ported, so Stage 11's deletion would have removed the last enforcement
of a documented contract.

It was not a no-op. Measured on a 4-row table:

    dimensions=[status], distinct_dimension_values=False,
    order=["amount:sum"]   ->  2 rows

The aggregate induced GROUP BY status, silently performing exactly the
dedup the flag turns off. The flag was accepted and ignored.

Two typed checks replace the legacy text re-parsing:

* `_reject_measure_refs_for_raw_rows` runs BEFORE binding, walking the
  typed parser's AST for AggCall / TransformCall / bare saved-measure
  refs. Pre-bind so the targeted, actionable error wins over the
  binder's generic "cannot resolve reference 'aov'" or "function 'sum'
  is not allowed in Mode B". Function-style aggregations are normalised
  through the slack helper first, as the legacy check did.
* A post-bind guard on the aggregate bucket in plan_query: in a raw-rows
  query, any aggregate slot can only have come from a filter or order
  item, since `measures` is rejected upstream. Catches whatever the text
  walk misses.

Consequence, user-approved: two DEV-1712 tests that asserted this shape
"still works" encoded the divergence and are re-pinned as reject tests.
Honouring both intents -- raw rows ordered by SUM(...) OVER (PARTITION
BY ...) -- is the better long-term answer and is filed as DEV-1736; it
is an addition rather than parity (main rejects these too), so it does
not block this merge.

The 14 tests agent-flagged as a typed-pipeline gap now pass unxfailed.
Full non-integration suite: 8594 passed, 5 skipped, 6 xfailed. Ruff clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy parser matched the scalar allowlist with
`name.lower() in SCALAR_PASSTHROUGH`; the typed parser matched exactly.
SQL function names are case-insensitive, so every SQL-cased formula
regressed: `COALESCE(revenue, 0)` and `Round(revenue, 2)` raised
UnknownFunctionError in both formulas and filters, where they worked on
main.

The lookup now lowercases, and ScalarCall.name is normalised on the way
in so `COALESCE(...)` and `coalesce(...)` intern to ONE key rather than
two slots computing the same value.

The other half of the same divergence -- the typed allowlist is missing
ceiling/mod/sign/trunc/greatest/least/ltrim/rtrim/substring -- is NOT
fixed here. Expanding a closed allowlist needs per-dialect emission and
execution evidence (greatest/least and mod are genuinely non-uniform),
so it is filed as DEV-1737 rather than added blind.

Full non-integration suite: 8596 passed, 5 skipped, 6 xfailed. Ruff clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex follow-up on ef5a738: `isinstance(spec.get("member"), str)` still
accepted `""`, so `{"cube_variables": {"note": {"member": ""}}}` counted as a
declaration and lost the brace-literal fast path — contradicting the doc's
"never mistaken for generated SQL". An importer always writes a parsed
identifier, so a member name is non-empty by construction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 1 hoisted the `SQLGenerator(...)` constructor out of `pytest.raises`,
but S5778 survived: `exp.Select()` was still constructed inside the block as
an argument, so Sonar still counted two potentially-throwing invocations.
Hoist the select too, leaving the call under test as the only invocation.

Sonar for this PR: 11 OPEN -> 0. CodeRabbit: no unresolved threads (the
remaining review-summary nitpick is stale — it cites the string-split guard
that round 1 replaced with the AST helper). Codex: no findings. CI: green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sonar python:S9073 (8×, tests/test_dev1732_frame_bound_filters.py) — split every
composite `assert X and Y` into one assertion per condition, each with its own
message. Worth doing properly rather than suppressing: these tests assert that
one bound left `_src` while another predicate stayed, so a composite failure
gave no signal about which half broke. The two join-alias assertions now name
which hop is missing.

Sonar python:S3776 (slayer/sql/generator.py) — NOSONAR on
`_build_where_having_from_planned` with justification. The complexity (30) is
pre-existing; DEV-1732 only added the `filters_override` list selection, but
changing the signature re-attributes the function as new code. Reaching ≤15
means refactoring shared WHERE/HAVING rendering that every query goes through —
out of scope here, and the file already carries 24 such suppressions.

CodeRabbit (tests/_engine_helpers.py) — `_extract_src_body` used `rfind`, which
returns -1 when the `LEFT JOIN (` anchor is absent and silently returns a slice
from an arbitrary offset. Unreachable with today's generator output; asserted so
a future formatting change fails clearly instead of pointing at the wrong text.

Codex — `_shifted_where_part`'s `time_columns` is now a REQUIRED keyword rather
than defaulting to `frozenset()`. `strip_frame_bounds` returns its input
unchanged for an empty set, so the default would have let a future caller
silently render every `date_range` into the shifted CTE — the exact 7b.3c
regression. Codex's two filed findings were both invalid on inspection (see PR
discussion), but this footgun is the real residue of the first one.

Full non-integration suite green: 8653 passed, 5 skipped, 6 xfailed. Ruff clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… sites

Every test is now off the legacy entry points except
tests/test_agg_render_spec_shim.py, which Stage 11 deletes outright.
~34 call sites across four files now run engine.execute(dry_run=True).

Assertions that read the EnrichedQuery object (cm.alias, cm.label,
enriched.filters) have no typed equivalent, so each was re-expressed
against generated SQL / the response, preserving intent:
* alias plumbing -> outer-SELECT projection alias list + _cm_ CTE column
* label -> resp.attributes[...].label, end-to-end
* declared type -> the CAST actually materialises on the typed path,
  where the legacy docstring recorded that it never did (stronger).

Five collision guards no longer raise, and this is correct rather than a
lost check. Under legacy's `orders.<hop>.<leaf>` scheme two measures
could construct the SAME public alias, so raising was the only way to
stop a silent merge. The naming module keys a renamed measure as
`orders.<name>` and an unrenamed cross-model one as
`orders.<hop>.<canonical>`, so they cannot collide by construction. The
surviving intent -- nothing silently merges -- is now asserted directly
and more specifically than the raise: exact alias list, uniqueness,
distinct _cm_ CTE count, and that each key carries its OWN aggregate
body. Verified independently by execution on the symmetric-swap case: a
measure named `revenue_avg` declared as revenue:sum returns 50 while
`revenue_sum` declared as revenue:avg returns 25 -- distinct, and the
bodies did not swap. The five tests are renamed off `_raises`, which no
longer described them.

DEV-1445 turns out to have landed on the typed pipeline: a filter via
either the bare user alias or the colon form routes to a HAVING inside
the _cm_ CTE. The test that asserted "raises until DEV-1445" is renamed
and re-pinned; its skipped colon-form sibling is un-skipped (the only
skip->pass change).

test_rewrite_fires_after_outer_projection_trim is deleted: it pinned
_apply_outer_projection_trim running before rewrite_emitted_sql, and
that trim does not exist on the typed pipeline (the planner owns the
public projection), so the ordering claim is unrepresentable rather than
merely relocated. Replaced by a test pinning the surviving contract --
BigQuery mangling reaches the FINAL public projection and resp.columns
decodes back to dotted keys.

Full non-integration suite: 8597 passed, 4 skipped, 6 xfailed. Ruff clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex found two shapes the entry-point relaxation opened up. Both were
regressions introduced by this PR, and both produced no error.

1. Silently dropped ORDER BY (stage_planner). `_expr_pending` can bind a
   top-level predicate such as `amount:sum in (1, 2)` to an `InKey`, which
   `_iter_slot_deps` treats as WHERE-inlined and never slots. `find_by_key`
   then missed and the order entry was DISCARDED — the query ran unsorted and
   returned the wrong rows with no error. Exactly the original `change(...)`
   failure mode, through a new door.

   Fixed structurally rather than by blocklisting `InKey`/`BetweenKey`: any
   order spec whose key has no slot now raises. A future key kind cannot
   silently reintroduce the class. A boolean composite that DOES slot
   (`a:sum > 1 and a:sum < 5`) keeps working — pinned by a control test so
   the new raise cannot be over-broad.

2. Row operand stringified inside a scalar call (generator). The
   ScalarCallKey renderer's trailing `else` coerced anything unrecognised
   with `str(a)`, so `coalesce(revenue:sum, quantity)` emitted

       COALESCE(SUM(orders.amount), 'path=() leaf=''quantity''')

   — structurally valid SQL comparing an aggregate against a debug-repr
   string literal. The arithmetic path already raised for the same operand.
   Now dispatches on the `_FrozenKey` base instead of a hand-listed subset,
   so any ValueKey routes through the recursive renderer and an unsupported
   operand hits the same terminal NotImplementedError; the `else` is left for
   genuine Python literals. Both paths pinned together so they cannot drift
   apart again.

Sonar for this PR: 0 issues, 0 hotspots, gate OK. CodeRabbit: no unresolved
threads. CI: green. 8643 passed non-integration, 544 passed integration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-explicit-time-column-filters-truncate-the

DEV-1732: frame-bound filters must not truncate trailing-window / shifted CTEs
…oach-expressions-crossing-joins-on-the' into egor/dev-1733-typed-pipeline-order-only-transformcomposite-refs-hidden

# Conflicts:
#	DECISIONS.md
#	docs/concepts/formulas.md
#	tests/integration/test_integration_windowed_measures.py
…wn detail

CodeRabbit (valid): the entry's opening sentence claimed every undeclared
transform / composite / windowed order target "materialises hidden, orders at
the outer wrap" — but the Materialisation paragraph in the same entry states
the opposite for one case: a composite whose operands include a `_cm_` / `_wm_`
value stays INLINE via `outer_composite_order_expressions`, precisely because
the combined path has no outer trim wrap and a materialised hidden column
there would leak as a public result column.

Rewrote the summary to name both paths up front. This is the log future
readers consult before changing this area, and the two failure modes it guards
against — plain-aggregate substitution, and hidden projections leaking — are
exactly what the wrong summary would invite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…der-only-transformcomposite-refs-hidden

DEV-1733: order-only transform / composite / windowed ORDER BY targets
…gestion

DEV-1608: Cube → SLayer ingestion (Stage 1)
Both branches amend the same plan-time ORDER BY classifier in plan_query,
improving DIFFERENT rows of its behaviour table. The merge takes both:

  * Phase 1 (local): GROUPED local row column   ValueError -> hidden <col>:max
  * DEV-1733 (origin): transform / composite    ValueError -> hidden slot

Neither touches the other's row, so nothing was traded away.

One ordering difference WAS material. DEV-1733 tests the joined-path check
before the grouping check, so it rejects an UNGROUPED joined row column;
Phase 1 tests grouping first, so that shape pulls its join (Law 1) and
split-emits. Phase 1's order wins: the raw-rows case is legal SQL (the row
IS the grain), it is pinned by executed values, and rejecting it was a
Stage-8 conservatism Phase 1 deliberately lifted. The joined + GROUPED
rejection both branches share is untouched and remains DEV-1735.

In order_entries, DEV-1733's loud raise for an unslotted order key is kept
and composed with Phase 1's order_key_remap lookup, so a remapped :max slot
resolves while any genuinely unslotted shape still fails loudly instead of
being silently dropped.

Two DEV-1733 tests pinned contracts this branch had already replaced (it
was written against a pre-Phase-1 base). Both are INVERTED, not deleted --
the treatment DEV-1733 itself gave the DEV-1501 tests it superseded -- in a
new TestSupersededByDev1703Phase1 class:

  * test_grouped_row_column_order_still_raises -> ..._max_wraps, now pinned
    by EXECUTED VALUES (open before paid on MAX(created_at) desc) rather
    than SQL shape, since the failure mode is a sort that silently no-ops.
  * test_order_only_transform_in_raw_rows_mode -> ..._rejected. Its
    rationale ("the typed pipeline deliberately does not enforce DEV-1543")
    is obsolete: this branch restored that check after the permissive
    behaviour was shown by execution to return wrong row counts. Its
    docstring also cited a test that does not exist.

tests/test_sql_generator.py: origin moved _norm / _join_aliases /
_extract_src_body into tests/_engine_helpers.py, which this branch had done
inline; kept the shared import and dropped both duplicates. Origin's
_noop_async is now dead here (Phase 2b removed the last enrich_query call
sites, which won the auto-merge) so it goes too.

8750 passed, 4 skipped, 5 xfailed; ruff clean.
Removes ~6800 lines: slayer/engine/enrichment.py (3369) and enriched.py
(316) entirely, plus the legacy subgraph in query_engine.py (3942 -> 2842)
and generator.py (11817 -> 10504).

REACHABILITY, MEASURED NOT ASSUMED. Before cutting, both legacy entry points
were instrumented with hit counters -- counters, not raises, so a swallowed
exception in a broad `except` could not hide a live path -- and the full
suite was run against the merged tree:

  _resolve_model_inner's named-query branch    0 hits
  _query_as_model                              1 hit (a test calling it directly)

That branch is unreachable because the typed pipeline resolves sibling
stages through _follow_sibling_chain in source_bundle.py, never through
_resolve_model. Confirmed statically too: _resolve_query_model's only
callers were itself and _query_as_model, _walk_join_chain's only callers
were the legacy resolvers, and NO module outside query_engine.py calls any
engine private method. The subgraph was closed, entered solely through that
dead branch.

DEV-1485 SAID AN ADAPTER WAS REQUIRED FIRST. It is not -- the branch it
wanted ported onto _expand_query_backed_model has no caller, so it is
deleted with everything else. Zero adapters, not one.

DEV-1485 ITEM 6 (gut core/formula.py) IS WRONG AND IS SKIPPED. parse_formula
did not die with enrichment: slayer/dbt/converter.py and
slayer/osi/converter.py both call it as a formula VALIDATOR (dbt categorises
its failures as `dangling_reference`). It returns FieldSpec, so the union
members -- AggregatedMeasureRef / ArithmeticField / TransformField /
MixedArithmeticField -- are its return-type contract and are equally live. A
per-symbol scan of formula.py found exactly one genuinely dead name,
FILTER_FUNCTIONS (an unused duplicate of _LIKE_INTERNAL_NAMES, no importer,
no __all__), which is removed.

Also collapsed now that their legacy mode is gone: _maybe_raise_schema_drift
loses its enriched= branch (both callers already pass touched_models) and
with it the orphaned _collect_models_touched; _build_agg loses its measure=
compat surface and _agg_render_spec_from_enriched; three zero-caller helpers
(_has_cross_model_filter, _is_windowed_measure, _window_referenced_aliases,
the last already superseded by ScopeFrame in DEV-1714). Three dicts annotated
Dict[str, "EnrichedMeasure"] actually hold AggRenderSpec (built by
_build_agg_render_spec_from_planned) -- stale annotations, retyped rather
than kept alive.

tests/parity_xfails.py + its conftest hook deleted. Per Codex F9 the hook was
inventoried first: it applies the strict-xfail markers and self-polices stale
keys, and does nothing else -- no ordering, filtering, or other marker logic
-- so with the registry empty (the DEV-1485 gate) both are no-ops.

Test migrations, all preserving the original invariant:
  * bigquery _query_as_model wrap  -> _expand_query_backed_model; the
    mangled-backtick invariant is unchanged, only the rename target differs
    (flat bind name, the documented virtual-model contract)
  * two strip_source_model_prefix tests -> end-to-end through the typed
    pipeline instead of poking _resolve_dimension_via_joins /
    _resolve_cross_model_measure; strictly better coverage, since execute()
    applies the strip itself
  * the ContextVar per-task recursion-guard test -> concurrent-expansion
    isolation, which is the property that actually mattered (the typed path
    threads _resolving as a parameter, so isolation is structural)
  * TestContextVarSafety deleted -- it asserted the migrated path never
    touches ContextVars that no longer exist
  * _walk_join_chain shim deleted (no production caller); its one test now
    calls path_resolution.walk_join_chain directly

Codex F11 audit beyond greps: all 164 slayer modules import cleanly in
isolated subprocesses; no dynamic/string-based imports of the deleted
modules; no deleted name in any __init__ or __all__.

8724 passed, 4 skipped, 5 xfailed; ruff clean.
…eality

The architecture docs were largely ABOUT the two-pipeline coexistence, so
this is a rewrite of those sections rather than a find/replace. Two of them
were already stale before Stage D:

  * engine-orchestration.md's "Where the legacy stack still runs (the
    deviation)" described query-backed expansion as running on
    _query_as_model in production. DEV-1452 Stage B had already moved it to
    the typed path. Replaced with a description of what actually happens,
    plus a collapsed historical note.
  * index.md deviation 3 said generate_from_planned "adapts back to
    EnrichedMeasure" via _synthesize_enriched_measure_from_planned. That
    function no longer exists -- DEV-1452 Stage A retyped the dialect
    helpers onto AggRenderSpec. Marked resolved.

Also: "Current state: two pipelines coexist" -> "one pipeline"; deviation 1
(the largest plan-vs-reality gap) marked resolved; sql-generation.md's
adapter section rewritten around AggRenderSpec with the EnrichedMeasure
hybrid demoted to a historical note.

Reader-facing flow diagrams in index.md, development.md and the
slayer-overview skill said SlayerQuery -> EnrichedQuery -> SQLGenerator.
Now PlannedQuery, described as what it is: typed value keys interned into
slots carrying resolved expression, join path and phase.

references.md keeps its `__` carve-out explanation -- users rely on that
behaviour -- but attributes it to the current owner (flat_name in
slayer/sql/naming.py, via _expand_query_backed_model) instead of
_query_as_model, and says "binding time" rather than "enrichment time".

In-code: 54 references to deleted symbols remain, and most are deliberately
kept -- "mirrors the legacy X" is design rationale that explains WHY code
looks the way it does, and deleting it would lose the why. Only claims in
the PRESENT tense were wrong, and all are fixed: query_engine's module
docstring and class docstring still advertised the _enrich() -> EnrichedQuery
flow as current; AggRenderSpec.agg_kwargs claimed to keep "the legacy
EnrichedMeasure shim" working; mcp/server.py referenced "a fresh
_query_as_model pass"; two stage_planner docstrings attributed the DOUBLE
coercion of joined dimension types to _query_as_model (the behaviour
survives -- it is the virtual-model wrap's `sc.type or DataType.DOUBLE`
fallback -- so the names were updated, not the explanation).

Remaining doc mentions are all past-tense or inside explicit "Historical:"
notes, kept so readers meeting older commits and issues can tell that the
two-pipeline period ended rather than assuming the code drifted.

No docs pages added or removed, so zensical.toml nav is unaffected.

DECISIONS.md records Stage D including the two DEV-1485 premises that turned
out to be wrong (the required adapter that had no caller; parse_formula
being live via the dbt/OSI converters).

8724 passed, 4 skipped, 5 xfailed; ruff clean.
…s tail

Two problems, both found by asking what recommend_root_model actually uses.

1. A TEST THAT PROVED NOTHING. test_inner_path_resolvable_by_engine_walker
   exists to check that the INNER hop recommend_root_model emits is walkable
   by the QUERY-TIME resolver -- the storage symmetry invariant. When the
   legacy stack went I rewired it from engine._walk_join_chain onto
   path_resolution.walk_join_chain, which WAS the query-time resolver when
   the test was written but is not any more: the typed pipeline walks join
   hops in binding.py against the resolved bundle, and walk_join_chain's only
   callers were the legacy resolvers just deleted. So the test pinned a
   function no query touches -- it would have kept passing while the property
   it names silently broke.

   It now feeds the recommendation straight back in as a query and asserts
   the join renders. That is the honest form of the same check and strictly
   stronger: it fails if any layer (binder, planner, generator) cannot
   traverse the hop. Uses the 3-item set from the sibling test, since a
   2-item set ties and breaks lexicographically to order_items, testing
   nothing about the hop.

   recommend_root_model itself is unaffected -- it uses JoinGraph +
   min_hops_root and never touched path_resolution.

2. THE DELETION LEFT A TAIL. My Stage D pass keyed on "method takes an
   `enriched` parameter", which misses legacy-only helpers that don't.
   Eleven were orphaned and are now deleted: _OrderColRef + _order_split_sql
   (DEV-1712's DECISIONS entry explicitly called these throwaway parity to be
   deleted with the legacy stack), _alias_prefixes, _filter_dotted_columns,
   _filter_references_available, _safe_parse_outer, _deps_available,
   _build_consecutive_periods_ctes, _build_self_join_column,
   _apply_placeholder_fill, plus _apply_order_limit_to_planned_sql_string
   which was already dead before this branch. Verified against pre-deletion
   backups to separate "orphaned by me" from "already dead".

   generator.py 10504 -> 10277, query_engine.py 2842 -> 2822.

I also deleted SlayerResponse._populate_columns and had to put it back: it
is a Pydantic @model_validator that fills `columns` from `data[0].keys()`, so
it is framework-invoked and shows zero AST references. 32 tests caught it
(empty response columns). That is the exact false-positive class I had
flagged one step earlier and then walked into anyway; the remaining eleven
were re-checked for decorators before trusting the scan again.

8724 passed, 4 skipped, 5 xfailed; ruff clean.
walk_join_chain + NoJoinError had no production caller left once the legacy
resolvers went -- the typed pipeline walks join hops in binding.py against
the resolved bundle, never through this module.

Deliberately widened past DEV-1485's file list, because leaving it was an
active trap rather than just dead weight: it had already absorbed one test
and made it look meaningful (see the previous commit -- the
recommend_root_model walker test pinned a function no query touches). A
module that nothing calls but that still LOOKS like the join resolver will
keep attracting tests like that.

Knock-on: _resolve_model's named_queries parameter is removed. Its only
remaining justification was the resolve_model callback contract that
walk_join_chain defined, and its one caller (get_column_types) never passed
it.

Dangling references updated to name the current owner (binding.py) rather
than the deleted one: join_graph.py's symmetry-invariant comment, its test's
module docstring, and the "single reference-resolution surface" bullet in
docs/concepts/references.md.

DECISIONS.md records the method note for the next deletion of this kind: the
zero-reference scan counts AST Name/Attribute refs, so framework-invoked
symbols (Pydantic validators, FastAPI routes) read as dead -- check
decorator_list before believing it.

8711 passed, 4 skipped, 5 xfailed; ruff clean.
…V-1703

Four conflicts in query_engine.py, all the same shape: main hardened Mode-A
variable substitution while this branch moved it onto the bundle and deleted
the legacy stack. Both halves are needed, so each resolution composes them.

  * imports -- unioned; ruff pruned what only the deleted legacy code used
  * substitution site -- keeps this branch's bundle-replacement mechanism
    (the typed planner/binder/cross-model renderers read models from the
    bundle, so the substituted copy must replace BOTH source_model and its
    entry in referenced_models) and adopts main's dialect-aware escaping
    (DEV-1727) plus its unconditional call (DEV-1730: a block-bearing model
    must run even with zero variables so {? ?} collapses to (1=1))
  * get_column_types -- keeps this branch's typed bundle path, adopts main's
    dialect-aware _render_probe_model

_resolve_datasource is hoisted above the substitution, since DEV-1727's
escaping needs the dialect there. Safe: substitution rewrites only the four
raw-SQL surfaces, never model.data_source.

DECISIONS.md: append-only, both sides kept.

Six test failures, both families fixed:

1. tests/test_cube_smoke.py (new from main) drove engine._enrich +
   SQLGenerator.generate(enriched=), both deleted here. Migrated to
   execute(dry_run=True), which exercises more of the path than the two
   legacy calls did and is what the module is actually for -- proving a
   converted Cube model survives resolution and SQL generation. Needed a
   DatasourceConfig saved, since the typed pipeline resolves the dialect
   from storage rather than taking it as a SQLGenerator argument.

2. tests/test_variables_planner.py pinned a contract DEV-1730 superseded.
   List values are now legal (the IN-pushdown primitive), so
   test_list_value_raises becomes test_list_value_renders_a_mode_b_tuple,
   pinning the trailing comma that Mode-B's `python` escaping emits --
   ('x',) is a tuple literal where ('x') is a parenthesised string, so the
   comma is what makes a single-element list parse as a collection. The
   dict/None rejections stand; only their message changed.

Verified the Mode-B x list-variable combination end-to-end (it is new: this
branch owns engine/variables.py, which does not exist on main, so nothing
had exercised escape="python" against a list before). The documented
spelling `region in ({regions})` renders IN ('paid', 'open') correctly.

esprima is a new dependency (Cube JS parsing); poetry install -E all.

9307 passed, 4 skipped, 5 xfailed; ruff clean.
…ander

Codex review of PR 263 flagged two module docstrings that still describe the
deleted legacy stack as current. My earlier scrub filtered for present-tense
keywords ("still", "until", "while the legacy"), which these did not match --
so it missed them, including one DEV-1485 D.4 lists explicitly.

  * slayer/sql/generator.py -- "converts EnrichedQuery to SQL ... works
    exclusively with EnrichedQuery objects ... done by the query engine's
    _enrich() step". Every type and entry point named there is deleted.
  * slayer/core/query.py -- "later converted into EnrichedQuery (see
    slayer/engine/enriched.py)", plus a Sphinx cross-ref to
    :func:`slayer.engine.enrichment.resolve_filter_columns`, a broken link to
    a deleted function.

Swept the rest of the same class while there (19 edits): "at enrichment" as
the name of a CURRENT phase becomes "at binding" across core/query.py,
core/models.py, core/enums.py and engine/normalization.py; refs.py stops
listing engine/enrichment.py as a consumer; query_engine.py's save_model
docstring no longer claims the legacy rewriters "still fire during
enrichment". Historical "mirrors the legacy X" comments are deliberately
KEPT -- they explain why code looks the way it does.

Chasing those references also turned up dead code the deletion orphaned:
column_expansion.py's ASYNC expander trio (_walk_path_to_target,
_process_column_node, expand_derived_refs -- 214 lines) plus the
ResolveModel type alias. They resolved join targets through
storage.get_model for the legacy path; their only remaining mentions were
two test docstrings and one cross-reference. The module's own section
comment had predicted exactly this ("The async path is removed with the rest
of enrichment in DEV-1452"), so it now says so in the past tense. No async
test file existed -- test_cross_model_derived_columns.py covered it via the
legacy path, which was migrated in Phase 2.

9307 passed, 4 skipped, 5 xfailed; ruff clean.
Gate was already OK (hotspots 0, duplication 1.4% vs 3%) and CodeRabbit had
no threads, but 11 issues were OPEN under pullRequest=263. Gate OK does not
mean no new issues -- the gate has no new-issues condition -- so these were
fixed rather than assumed clean.

Three are mine from Phase 1 and the DEV-1733 merge:

  * S3626 stage_planner.py:1227 -- redundant `continue`. Real, and introduced
    by my merge resolution: the transform/composite branch became
    comment-only when DEV-1733 landed, leaving the `continue` as the last
    statement in the loop body. Removed.
  * S3776 _expr_has_measure_ref -- complexity 25/15. The closure-plus-nonlocal
    walker is replaced by plain recursion over a new _iter_expr_children
    generator. Same traversal, and the child-enumeration rule (attribute-
    driven, skip str/bool leaf payloads) is now stated in one place.
  * S3776 _reject_measure_refs_for_raw_rows -- complexity 27/15. Split along
    the seam the docstring already described: _reject_measure_refs_in_filters
    and _reject_measure_refs_in_order, with the funcstyle-rewrite-then-parse
    dance extracted to _parse_order_formula.

Three S5778 in test_transforms_planner.py: each pytest.raises block wrapped
BOTH parse_expr and bind_expr, so a parse regression would have been
mistaken for the binder rejection each test is actually about. parse_expr is
hoisted out; the block now contains only the call under test.

Five S9073 composite assertions split into one assertion each
(test_sql_generator x2, test_carrier_scope_matrix x2, test_scope_check x1) --
each half now reports which condition failed instead of just "assert False".

9307 passed, 4 skipped, 5 xfailed; ruff clean.
Codex second pass. Deleting the async expander trio left the three surviving
sync functions describing themselves as "Sync mirror of :func:`<deleted>`" --
Sphinx links resolving nowhere, and the production implementation documented
as a copy of code that no longer exists. Each now states its own contract:
what it returns, and what it does on an unresolvable/opaque alias path
(_walk_path_to_target_sync), rewritten-vs-left-alone (_process_column_node_sync),
and chain recursion + ColumnCycleError (expand_derived_refs_sync).

test_cross_model_derived_columns.py's cycle test claimed to pin "the same
ValueError that expand_derived_refs raises" and to exercise detection
"through _enrich" -- both deleted. It goes through expand_derived_refs_sync
now; the cycle-detection contract it actually protects is unchanged, and the
docstring says so.

9307 passed, 4 skipped, 5 xfailed; ruff clean.
My first pass hoisted only parse_expr out of the three pytest.raises blocks
in test_transforms_planner.py. Sonar still flagged them, correctly: _scope()
and _bundle() are invocations inside the block as well, so a fixture-builder
failure would still be mistaken for the binder rejection each test is about.
Both are now bound before the block, leaving bind_expr as the only call that
can throw inside it.

The same pattern appears elsewhere in this file but Sonar does not flag it --
those lines are not new in this PR -- so they are left alone rather than
widening the diff.

9307 passed, 4 skipped, 5 xfailed; ruff clean.
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@ZmeiGorynych
ZmeiGorynych merged commit e7d078c into egor/dev-1450-principled-redesign-of-syntax Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants